Skip to content

fix(requesting-code-review): correct fixture line refs (post-merge Copilot finding)#6

Merged
intel352 merged 1 commit into
mainfrom
fix/skills-pr1-fixture-line-refs
Apr 25, 2026
Merged

fix(requesting-code-review): correct fixture line refs (post-merge Copilot finding)#6
intel352 merged 1 commit into
mainfrom
fix/skills-pr1-fixture-line-refs

Conversation

@intel352
Copy link
Copy Markdown
Contributor

Summary

Copilot surfaced two line-ref errors in the expected-new-result.md fixture after PR #4 was already merged. This PR corrects them.

The diff hunk is @@ -10,6 +10,12 @@ func (d *Dispatcher) Create(req Request) error {. The Create function declaration appears as the @@ context label at line 10. Actual hunk content starts at line 11, so:

Reference Old (wrong) New (correct)
Create's "kind": d.kind line 11 line 12
Inspect method start (Finding 1 Location) lib/dispatcher.go:17 lib/dispatcher.go:18
Delete's "kind": d.kind line 26 line 27

Three locations updated: bug-class scan citation, Finding 1 Location field, Finding 1 What's wrong text.

Verify Regression Invariant

This is a documentation-only fixture change — no production code path. Revert-and-restore proof applies to the correctness of the line numbers themselves:

  • With fix reverted: diff.patch hunk +10,12 counts "kind": d.kind in Create at new-file line 11 (wrong — the declaration is line 10, body starts at 11, "kind" is at 12).
  • With fix applied: "kind": d.kind in Create correctly cited as line 12, matching the hunk arithmetic.

Test plan

  • diff.patch hunk verified manually: @@ -10,6 +10,12 @@ → line 10 = func Create declaration → line 12 = "kind": d.kind in Create body → line 18 = func Inspect → line 27 = "kind": d.kind in Delete
  • All three refs in expected-new-result.md updated and verified
  • git diff origin/main..HEAD eyeballed line-by-line before push

🤖 Generated with Claude Code

…pilot finding)

Hunk @@ -10,6 +10,12 @@ places the Create function declaration at line 10
(shown as @@ context label), so content starts at line 11:

  line 11: return d.client.Send("create", ...)
  line 12: "kind": d.kind              ← Create's kind ref (was 11)
  ...
  line 18: func Inspect(...)           ← Inspect location (was 17)
  ...
  line 27: "kind": d.kind              ← Delete's kind ref (was 26)

Fixes three stale refs in expected-new-result.md: bug-class scan
citation (line 10), Finding 1 Location (line 30), and Finding 1
What's wrong (line 31).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 25, 2026 02:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Corrects post-merge line-reference inaccuracies inside the expected-new-result.md fixture for the sibling-asymmetry requesting-code-review skill, keeping the fixture’s citations consistent with the diff.patch hunk line arithmetic.

Changes:

  • Update bug-class scan text to reference Create "kind": d.kind at line 12 and Delete "kind": d.kind at line 27.
  • Update Finding 1’s Location and “What’s wrong” text to reference Inspect starting at lib/dispatcher.go:18 and align sibling line references accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@intel352 intel352 merged commit 18114cb into main Apr 25, 2026
11 checks passed
intel352 added a commit that referenced this pull request Apr 25, 2026
…der syntax + team-lead graph label

- Switch {N}/{team-name} curly-brace placeholders to <N>/<team-name> angle-bracket
  style in implementer-prompt.md and spec-reviewer-prompt.md, consistent with
  all other skill templates
- Update DOT graph node label from "Team Lead" to "team-lead" to match the
  SendMessage recipient identifier used throughout the workflow text
- Add fixture invariant #6 + verify command for curly-brace placeholder detection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352 added a commit that referenced this pull request Apr 25, 2026
* skill(subagent-driven-development): DRY team conventions to agents/team-conventions.md

Extract repeated boilerplate from implementer/spec-reviewer/code-reviewer
prompts into a single shared file. Each prompt template now references
agents/team-conventions.md instead of inlining TDD discipline, self-review
checklist, review protocol, and sanitization rules.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* skill(subagent-driven-development): fixture — DRY conventions before/after

Documents the before/after of extracting team conventions from inline
prompt boilerplate to agents/team-conventions.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address 5 Copilot round-1 findings

A: conventions.md code-reviewer — bug-class checklist + verdict vocabulary
   now reference skills/requesting-code-review/SKILL.md explicitly.
B: conventions.md implementer — version-skew audit now references
   skills/finishing-a-development-branch/SKILL.md Step 1c.
C: implementer-prompt.md — same version-skew audit Step 1c reference.
D: code-quality-reviewer-prompt.md — split adversarial framing from
   bug-class checklist + verdict vocabulary; latter now point to
   skills/requesting-code-review/SKILL.md.
E: conventions.md code-reviewer — adversarial framing exception clause
   preserved: if fewer than 3 issues found, document every bug-class
   check run rather than manufacturing issues.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address 2 Copilot round-2 findings

A: implementer-prompt.md — restore explicit "DM spec-reviewer when ready"
   step so the spec-compliance gate isn't stalled after implementation.
B: SKILL.md — fix stale "checklist below" reference in implementer workflow
   step 4; now points to agents/team-conventions.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address 2 Copilot round-3 findings

A: implementer-prompt.md — DM team-lead with branch/commit + "ready for
   merge" (not PR link; PR is created by team-lead via finishing-a-branch
   after all tasks complete, not by the implementer).
B: SKILL.md code-reviewer section — split bug-class checklist + verdict
   vocabulary to their canonical source (skills/requesting-code-review/
   SKILL.md); team-conventions only hosts adversarial framing + output format.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address round-4 finding + tighten fixture

SKILL.md implementer Team Conventions block: replace enumeration that
implied "self-review checklist" is inline with explicit "all rules are
defined in agents/team-conventions.md, not repeated here"; also sync
version-skew audit pointer to finishing-a-development-branch Step 1c.

Fixture: add 5 invariants the "after" state must hold, including:
- no "checklist below" / "see below" / "inlined here" language
- every version-skew audit reference points to Step 1c
- every bug-class / verdict reference points to requesting-code-review
- adversarial-framing exception clause preserved verbatim
Verify command excludes test-fixtures dir to avoid self-matching.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address 2 Copilot round-5 findings

A: fixture verify command — switch to grep -rEn with ERE; replace \b
   (non-portable) with ([[:space:][:punct:]]|$) for reliable Self-Review
   boundary match across grep implementations.
B: team-conventions.md sanitization rule — tighten "no version references"
   to "no specific internal project/company/product-version/incident
   references"; explicitly allow dependency/runtime/tooling version numbers
   needed for accurate technical guidance (version-skew audit, pin refs).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address round-6 finding + fixture ref check

code-quality-reviewer-prompt.md: fix broken path "requesting-code-review/
code-reviewer.md" → "skills/requesting-code-review/code-reviewer.md"
(missing skills/ prefix; file exists at the correct path).

Fixture: add 2 new invariant checks:
- Dead skill file refs: grep all skills/*.md references, verify each
  resolves to an existing file (catches missing-prefix bugs like this one).
- Bare requesting-code-review refs: grep for refs without skills/ prefix
  (both checks exclude test-fixtures dir to avoid self-matching).
All three invariant checks PASS on current tree.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): normalize team-lead spelling across all role prompts

Round-7 fix: use hyphenated team-lead consistently with agents/team-conventions.md
and implementer-prompt.md usage pattern, avoiding ambiguity with the Agent Teams
role identifier.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address round-9 findings — placeholder syntax + team-lead graph label

- Switch {N}/{team-name} curly-brace placeholders to <N>/<team-name> angle-bracket
  style in implementer-prompt.md and spec-reviewer-prompt.md, consistent with
  all other skill templates
- Update DOT graph node label from "Team Lead" to "team-lead" to match the
  SendMessage recipient identifier used throughout the workflow text
- Add fixture invariant #6 + verify command for curly-brace placeholder detection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): fix fixture grep to catch start-of-line bare refs

Round-10 finding: [^/]requesting-code-review/ requires a preceding character,
so a bare ref at the start of a line would be missed. Switch to
(^|[^/])requesting-code-review/ to cover both cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): remove parenthetical rule enumeration — point to team-conventions only

Round-11 findings: both implementer-prompt.md and the embedded implementer
prompt in SKILL.md enumerate convention details (TDD, regression-invariant,
etc.) that are already centralized in agents/team-conventions.md. This
re-inlining contradicts the DRY goal of the refactor. Trim to a simple
pointer; the single source of truth is the conventions file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(subagent-driven-development): address round-12 findings — PLAN_REFERENCE field + grammar

- Add PLAN_REFERENCE field to code-quality-reviewer dispatch template to match
  the {PLAN_REFERENCE} placeholder in skills/requesting-code-review/code-reviewer.md
  (distinct from PLAN_OR_REQUIREMENTS used in the instructions section)
- Fix grammar in agents/team-conventions.md: "Shared rules every team agent"
  → "Shared rules that every team agent"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352 added a commit that referenced this pull request Apr 25, 2026
… plan+design

Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no
matches — looks like failure. Fix: add || true, update expected output description.

Finding #2 (design:142): described markers as "HTML-comment-shaped" but the
syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax
that most markdown renderers treat as unknown HTML tags".

Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but
table uses sentinel value host-pass-through. Fix: prose now matches sentinel.

Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate
has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing.

Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file
and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop
the file from the task entirely.

Finding #6 (plan:141): guard script ran grep twice per token, reported line
numbers from the stripped (AWK-processed) string rather than the original file.
Fix: single-pass AWK that emits "LINENO:content" preserving original line
numbers; single grep pass with || true.

Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact
single-host tag form, not comma-separated lists like <host: codex, claude-code>.
Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line,
handling any comma-separated host list.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352 added a commit that referenced this pull request Apr 25, 2026
… plan+design

Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no
matches — looks like failure. Fix: add || true, update expected output description.

Finding #2 (design:142): described markers as "HTML-comment-shaped" but the
syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax
that most markdown renderers treat as unknown HTML tags".

Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but
table uses sentinel value host-pass-through. Fix: prose now matches sentinel.

Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate
has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing.

Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file
and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop
the file from the task entirely.

Finding #6 (plan:141): guard script ran grep twice per token, reported line
numbers from the stripped (AWK-processed) string rather than the original file.
Fix: single-pass AWK that emits "LINENO:content" preserving original line
numbers; single grep pass with || true.

Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact
single-host tag form, not comma-separated lists like <host: codex, claude-code>.
Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line,
handling any comma-separated host list.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352 added a commit that referenced this pull request Apr 25, 2026
…ign docs (#13)

* fix(pr12-round2): address 7 Copilot findings on cross-LLM portability plan+design

Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no
matches — looks like failure. Fix: add || true, update expected output description.

Finding #2 (design:142): described markers as "HTML-comment-shaped" but the
syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax
that most markdown renderers treat as unknown HTML tags".

Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but
table uses sentinel value host-pass-through. Fix: prose now matches sentinel.

Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate
has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing.

Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file
and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop
the file from the task entirely.

Finding #6 (plan:141): guard script ran grep twice per token, reported line
numbers from the stripped (AWK-processed) string rather than the original file.
Fix: single-pass AWK that emits "LINENO:content" preserving original line
numbers; single grep pass with || true.

Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact
single-host tag form, not comma-separated lists like <host: codex, claude-code>.
Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line,
handling any comma-separated host list.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(cross-llm-plan): address 14 Copilot round-1 findings on plan+design

Plan doc (cross-llm-portability.md):
- Add lowercase sonnet/opus/haiku to forbidden-tokens list and explain why
- Add rationale for not yet including AskUserQuestion/Agent (PRs B-D)
- Remove tests/skill-content-grep.sh from ALLOWED_FILES (tests/ not scanned)
- Fix AWK skip to match only exclusive <host: claude-code> blocks, not
  multi-host tags; update inline comment accordingly
- Remove dead fail=1 inside while-pipe subshell; rely solely on $tmp check
- Add || true to all 7 skill-scoped grep verification commands (lines
  443, 517, 626, 694, 764, 844, 888) — grep exits 1 on no-match (PASS case)

Design doc (cross-llm-portability-design.md):
- Fix broken path: subagent-driven-development.md →
  skills/subagent-driven-development/SKILL.md
- Remove false gpt-5.x claim from grep guard description; clarify that
  only Claude-Code brand names are guarded, Codex names use <host:> markers
- Replace "Open questions deferred to implementation" with resolved decisions:
  AWK guard approach, angle-bracket marker syntax, OpenCode tool mapping,
  Cursor support depth scope

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(cross-llm-plan): address 7 Copilot round-2 findings on plan + design docs

- set -euo pipefail (was set -u only)
- Fix stray || true in step-title prose and GHA step name
- Verification commands capture guard output before grepping to avoid masking execution errors
- Fix prose claiming gpt-5.x tokens are guarded (guard only covers Claude brand names)
- Design doc: clarify angle-bracket marker advantage is readability in rendered Markdown

* fix(cross-llm-plan): add end-anchor to AWK skip regex so multi-host blocks are never skipped

Agent-Logs-Url: https://github.com/GoCodeAlone/claude-superpowers/sessions/24d5a30f-f7b3-43e7-83dd-60c470d0f4a4

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

* Update docs/plans/2026-04-25-cross-llm-portability.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants